fix(auth): when auth.enforce is false, still run auth checks as dry-run#359
fix(auth): when auth.enforce is false, still run auth checks as dry-run#359matt-codecov wants to merge 1 commit intomainfrom
Conversation
matt-codecov
force-pushed
the
matt/auth-dry-run
branch
from
065ba40 to
64aa65f
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| Some(auth) => auth.assert_authorized(perm, context), | ||
| None => Ok(()), | ||
| } | ||
| .inspect_err(|err| err.log(Some(perm), Some(context.usecase.as_str()))); |
There was a problem hiding this comment.
Double debug logging on authorization check failures
Low Severity
When AuthContext::assert_authorized in context.rs fails, it emits a tracing::debug! with message "Authorization failed". Then inspect_err in assert_authorized calls err.log(), which emits a second tracing::debug! with message "Authorization failure" and increments a counter. The counter is new and valuable, but the debug log is redundant — every per-operation auth failure now produces two debug entries with overlapping information.
Additional Locations (1)
There was a problem hiding this comment.
this is incorrect - context.rs logs for verification failure and assert_authorized() logs for insufficient perms, etc
2 participants
currently when
auth.enforceis false we skip auth entirely. in order to safely roll auth out, we want to change that. with this PR, auth logic will always run andauth.enforcesimply controls whether failure should result in the request being rejectedthere is still some danger associated with rolling out auth (startup failure if we mess up secret mounting, for example) but now we'll be able to ensure sentry/launchpad/relay auth integration is working end-to-end before actually flipping the switch